Skip to content

feat(relay): JSON /healthz with version and connection counts (#10)#18

Merged
ilmoniemi merged 3 commits into
mainfrom
feature/10
May 8, 2026
Merged

feat(relay): JSON /healthz with version and connection counts (#10)#18
ilmoniemi merged 3 commits into
mainfrom
feature/10

Conversation

@ilmoniemi
Copy link
Copy Markdown
Contributor

What

Replace the plain-text "ok\n" /healthz handler with a JSON response containing five fields: status, version, connected_binaries, connected_phones, uptime_seconds. Endpoint remains unauthenticated and serves identically to every HTTP method (no behavioural change there). Adds Cache-Control: no-store so intermediaries can't serve stale counts.

The handler is a factory (relay.NewHealthzHandler) returning http.Handler — keeps the door open for adding per-handler state later (logger, clock injection) without rewriting the call site. main now also captures startedAt after flag parsing and constructs a *relay.Registry; the registry is idle in v1 (no WS upgrade handlers yet) but holding it now means #4/#5/#16 don't have to refactor main.

Issue

Closes #10.

Testing

  • internal/relay/healthz_test.go adds two tests:
    • TestHealthz_ResponseShape — covers status code, both response headers, all five JSON keys present and well-typed, body under 200 bytes, and a uptime ≥ 30s when constructed with a 30s-old startedAt. Uses a generic map[string]json.RawMessage decode in addition to the typed decode so a Go field rename that broke the wire contract would still fail.
    • TestHealthz_TracksRegistryState — populates a registry with 2 binaries and 5 phones across 2 server-ids, asserts the handler reports those counts.
  • go test -race ./... clean.
  • go vet ./... clean.
  • go build ./cmd/pyrycode-relay clean.

Architecture compliance

  • File layout matches the spec: new internal/relay/healthz.go and internal/relay/healthz_test.go, single edit to cmd/pyrycode-relay/main.go.
  • healthzResponse is unexported; field order matches the AC's wire-key order.
  • Handler does one Counts() call per request and releases the registry RLock before any response I/O (matches the established "copy under lock, do slow work outside" pattern).
  • time.Since floored at zero — defence-in-depth per the spec, not observable today but cheap to keep.
  • json.Marshal of a fixed primitive struct cannot fail; discarded error documented inline. Marshal-then-write keeps the response atomic.
  • No new dependencies (stdlib encoding/json, net/http, time).
  • Security review items honoured: no caller input read, no echoed bytes, aggregate counts only (no per-server-id breakdown), Cache-Control: no-store set, header values are constant strings.

🤖 Generated with Claude Code

ilmoniemi added 2 commits May 8, 2026 21:40
Replace the plain-text "ok" handler with a JSON response carrying status,
version, current binary/phone counts, and uptime in seconds. Endpoint
remains unauthenticated by design; aggregate counts are an explicit
operational tradeoff (no per-server-id breakdown).

The new handler lives in internal/relay/healthz.go as a factory
returning http.Handler so future per-handler state (logger, clock) can
be added without changing the call site. main now constructs startedAt
post flag-parsing and a Registry handle (idle until WS upgrade tickets
land) and wires both into the handler. Sets Cache-Control: no-store to
keep intermediaries from serving stale counts.

Tests cover response shape (status, headers, all five JSON keys, body
under 200 bytes) and registry-state tracking (claimed binaries and
registered phones reflected in the counts).
@ilmoniemi
Copy link
Copy Markdown
Contributor Author

Code Review: #10

Decision: PASS

Findings

  • [NIT] internal/relay/healthz_test.go:32 — len(body) >= 200 is checked against a body of ~85 bytes (version="test-version", counts=0). The assertion is correct and useful as a regression guard, but it doesn't exercise anything close to the spec's worst-case (~135 bytes). A second assertion with high-cardinality counts would tighten the budget — non-blocking; the present check still catches a structural regression.
  • [NIT] internal/relay/healthz.go:36 — request param r is unused (handler ignores method/headers/body by design). Idiomatic Go keeps it named in handler signatures, so leaving it as r is the right call; flagging only because a future linter pass might suggest _.

Summary

The diff matches the architect's spec exactly: new internal/relay/healthz.go with unexported healthzResponse (field order pinned, JSON tags match the AC), NewHealthzHandler factory returning http.Handler, single Counts() call released before any I/O, Cache-Control: no-store set, time.Since floored at zero as documented defence-in-depth. main.go captures startedAt after the --version early-return (correct semantics — "began serving" rather than "binary started") and constructs the registry once so #4/#5/#16 won't have to refactor main.

Doc-comment on NewHealthzHandler names the unauthenticated-by-design intent and the concurrency contract (no per-request state, no goroutines, RLock released before I/O). The discarded json.Marshal error is documented inline; the marshal-then-write pattern is correctly justified for response atomicity.

Tests cover all 9 AC bullets across two cases. TestHealthz_ResponseShape asserts status code, both response headers, body-size budget, typed decode of all fields, and a separate map[string]json.RawMessage decode that would catch a Go-field rename that broke the wire contract — nice belt-and-suspenders. TestHealthz_TracksRegistryState correctly reuses the existing in-package fakeConn helper without re-export.

Security-sensitive label honoured: the architect's spec contains a ## Security review section with categories walked (trust boundaries, info disclosure, probe amplification, cache poisoning, races, header injection, method confusion, timing channels, supply chain) and a PASS verdict. Diff implementation matches every claim made in that review (no caller input reaches the response, headers are constant strings, aggregate counts only via Counts(), no new deps).

go vet ./... clean. go test -race ./internal/relay/... passes. CI test job green; security job (gosec/govulncheck) was still running at review time but there are no new dependencies and no caller-controlled string handling, so a clean result is the expected outcome.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ilmoniemi ilmoniemi merged commit 77d27e1 into main May 8, 2026
2 checks passed
@ilmoniemi ilmoniemi deleted the feature/10 branch May 8, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

relay: /healthz returns version + connected-binary count + connected-phone count (JSON)

1 participant